Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UI Framework] Add isReadOnly prop to KuiCodeEditor. #14804

Merged

Conversation

cjcenizal
Copy link
Contributor

No description provided.

@nreese
Copy link
Contributor

nreese commented Nov 7, 2017

Thanks for moving this into the ui framework.

One thing I could not figure out - is there any way to avoid showing the cursor until the user interacts with the editor? Even in readonly mode, the cursor is placed at the end. It would be nice if there was no cursor until the user clicked in the text for copy selection.

@timroes
Copy link
Contributor

timroes commented Nov 7, 2017

@nreese You can use the following CSS to hide the inactive cursor:

.ace_hidden-cursors { opacity:0 } 

I am not sure if we want to do this in general for the read only mode? Maybe we could create another flag that sets a CSS class on the wrapper, and only apply this when this flag is set? But I would also be fine always hiding read only inactive cursors, if you think it's better.

Copy link
Contributor

@timroes timroes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nreese
Copy link
Contributor

nreese commented Nov 7, 2017

I think it makes sense to hide the inactive cursor since the editor is readonly. I find the cursor distracting. It invites you to start typing - which is not allowed

@cjcenizal
Copy link
Contributor Author

We need to keep the cursor visible, at least while the user is interacting with the editor. This will allow people who are interacting via keyboard to select text.

I looked through the docs, but couldn't find a way to hide the cursor. The best I could do was place the cursor at the beginning of a read-only code editor. This is a little less obtrusive, and probably more usable since you're more likely to want to select the code from the beginning than the end. Can you take another look @nreese ?

@cjcenizal
Copy link
Contributor Author

cjcenizal commented Nov 7, 2017

Oh wait, I can use Tim's CSS to hide the cursor until the editor has received focus. One sec...

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@cjcenizal
Copy link
Contributor Author

@timroes @nreese Sorry to pester you but I made a significant change which could use your 👀 again. Now I show the accessibility prompt for the code editor even if it's in read-only mode. I also hide the cursor until you focus the editor, which you should like Nathan!

if (isReadOnly) {
// Put the cursor at the beginning of the editor, so that it doesn't look like
// a prompt to begin typing.
filteredCursorStart = -1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this logic still needed now that css is used to hide the cursor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it still makes sense to put the cursor at the beginning for people who are using the keyboard to interact with the content. I'm guessing they'd want to copy/paste it and it makes sense to me intuitively to do that from start to end rather than the reverse.

@cjcenizal
Copy link
Contributor Author

jenkins, test this

@nreese
Copy link
Contributor

nreese commented Nov 8, 2017

@cjcenizal Thanks for hiding the cursor in readonly mode. This looks great.

lgtm once CI passes

@cjcenizal cjcenizal merged commit b6b0e61 into elastic:master Nov 9, 2017
@cjcenizal cjcenizal deleted the improvement/code-editor-read-only branch November 9, 2017 17:22
cjcenizal added a commit to cjcenizal/kibana that referenced this pull request Nov 9, 2017
* Add isReadOnly prop to KuiCodeEditor.
* Place cursor at beginning of code editor if it's read-only.
* Surface prompt for isReadOnly mode. Only show cursor when the editor is focused.
cjcenizal added a commit that referenced this pull request Nov 9, 2017
* Add isReadOnly prop to KuiCodeEditor.
* Place cursor at beginning of code editor if it's read-only.
* Surface prompt for isReadOnly mode. Only show cursor when the editor is focused.
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Nov 20, 2017
* Add isReadOnly prop to KuiCodeEditor.
* Place cursor at beginning of code editor if it's read-only.
* Surface prompt for isReadOnly mode. Only show cursor when the editor is focused.
chrisronline pushed a commit to chrisronline/kibana that referenced this pull request Dec 1, 2017
* Add isReadOnly prop to KuiCodeEditor.
* Place cursor at beginning of code editor if it's read-only.
* Surface prompt for isReadOnly mode. Only show cursor when the editor is focused.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants